fix(tests): fix reconfiguration completion detection #12117
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hanshuebner
force-pushed
the
fix/reconfiguration-completion-detection
branch
2 times, most recently
from
8538677 to
2cee947
Compare
hanshuebner
force-pushed
the
fix/reconfiguration-completion-detection
branch
from
5c190ea to
2f8eb74
Compare
hanshuebner
force-pushed
the
fix/reconfiguration-completion-detection
branch
from
b4a4204 to
622e9e4
Compare
Previously, the transaction ID that was reported was from before the admin API request was finished. Also, invalidations could sometimes be delayed long enough to cause a conditional request to be executed even though a subsystem has not been rebuilt. This change fixes the behavior in multiple ways: - Invalidations are invoked from the DAO:post_crud_event in a synchronous fashion rather than having event handlers invalidate. This ensures that when the admin API request finishes, the cache is invalidated as well. - The transaction ID is saved to the nginx context in `DAO:post_crud_event` and set in the response header in the `Kong.admin_header_filter` function. - The current transaction ID is retrieved by the data plane from the shared cache instead of from Postgres. Previously, every iteration of the rebuild cycle incremented the current transaction ID by one, making debugging more difficult.
hanshuebner
force-pushed
the
fix/reconfiguration-completion-detection
branch
from
a5e2e8e to
d2d4242
Compare
dndx
added
the
pr/discussion
|
Hello @hanshuebner , temporarily converting this to Draft so we can wait for the updated design doc. Let's take a bit more time and make sure to execute it properly. Please also briefly describe the issue encountered with the first design in the updated design doc. cc @hbagdi , will ping you once the updated design doc is up. |
|
@dndx I've updated https://docs.google.com/document/d/1SOFb8eQWwBgWlLPppQHAN7kSg9eHkDirjgDGAmOPRro/edit according to your request. |
|
@hbagdi says that it is too risky to change the invalidation behavior to support testing better. Closing this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Previously, the transaction ID that was reported was from before the admin API request was finished. Also, invalidations could sometimes be delayed long enough to cause a conditional request to be executed even though a subsystem has not been rebuilt.
This change fixes the behavior in multiple ways:
Invalidations are invoked from the DAO:post_crud_event in a synchronous fashion rather than having event handlers invalidate. This ensures that when the admin API request finishes, the cache is invalidated as well.
The transaction ID is saved to the nginx context in
DAO:post_crud_eventand set in the response header in theKong.admin_header_filterfunction.The current transaction ID is retrieved by the data plane from the shared cache instead of from Postgres. Previously, every iteration of the rebuild cycle incremented the current transaction ID by one, making debugging more difficult.
Checklist
changelog/unreleased/kongorskip-changeloglabel added on PR if changelog is unnecessary. README.mdIssue reference
KAG-3195